Skip to content

fix: replace hardcoded 60s timeout in deploy_model() with INFERENCE_TX_TIMEOUT#220

Merged
adambalogh merged 1 commit intoOpenGradient:mainfrom
amathxbt:fix/deploy-model-hardcoded-timeout
Apr 1, 2026
Merged

fix: replace hardcoded 60s timeout in deploy_model() with INFERENCE_TX_TIMEOUT#220
adambalogh merged 1 commit intoOpenGradient:mainfrom
amathxbt:fix/deploy-model-hardcoded-timeout

Conversation

@amathxbt
Copy link
Copy Markdown
Contributor

🐛 Bug: Hardcoded Magic-Number Timeout in deploy_model()

File: src/opengradient/client/alpha.pydeploy_model()

What's broken

Every wait_for_transaction_receipt call in this file consistently uses the defined constants INFERENCE_TX_TIMEOUT = 120 or REGULAR_TX_TIMEOUT = 30, except for one outlier in deploy_model() (line 318) which uses a hardcoded 60:

# deploy_model() — line 318
tx_receipt = self._blockchain.eth.wait_for_transaction_receipt(tx_hash, timeout=60)  # ← magic number!

Contract deployment is the most resource-intensive transaction type (deploys full bytecode on-chain). It should use INFERENCE_TX_TIMEOUT = 120 at minimum, not an arbitrary 60s that:

  1. Is inconsistent with every other call in the same file
  2. Can cause premature TimeExhausted failures on congested networks
  3. Provides no single place to tune the timeout if needed

Fix

tx_receipt = self._blockchain.eth.wait_for_transaction_receipt(tx_hash, timeout=INFERENCE_TX_TIMEOUT)

Impact

Before After
deploy_model() times out after 60s (arbitrary) Uses INFERENCE_TX_TIMEOUT (120s), consistent with all other heavy transactions
Random failures on slow/congested networks Predictable, tunable timeout
Magic number obscures intent Self-documenting constant

Files Changed

  • src/opengradient/client/alpha.py

@amathxbt
Copy link
Copy Markdown
Contributor Author

@adambalogh

@adambalogh adambalogh merged commit bcde6db into OpenGradient:main Apr 1, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants